Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 4 | Implement cowsay in Python#267
Open
HassanOHOsman wants to merge 25 commits intoCodeYourFuture:mainfrom
Open
Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 4 | Implement cowsay in Python#267HassanOHOsman wants to merge 25 commits intoCodeYourFuture:mainfrom
HassanOHOsman wants to merge 25 commits intoCodeYourFuture:mainfrom
Conversation
…esult of the arguments with spaces in between
…an end user can select from
HassanOHOsman
added
Needs Review
LonMcGregor
added
the
Review in progress
|
Can you ignore the .venv file? It is difficult to review this as there are too many files in the PR. thanks |
LonMcGregor
added
Reviewed
HassanOHOsman
added
Needs Review
Author
Hi! I've just ignored it. Please let me know if there's anything else needs changing. Appreciate your help! :) |
LonMcGregor
reviewed
Jan 12, 2026
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start, i've left some comments where you could improve further
implement-cowsay/cow.py
Outdated
| import sys | ||
| import argparse | ||
|
|
||
| listed_animals = [listed_animal for listed_animal in dir(cowsay) if callable(getattr(cowsay, listed_animal)) and not listed_animal.startswith("__") and listed_animal not in ["draw", "func", "get_output_string", "CowsayError"]] |
There was a problem hiding this comment.
This might work, but looking over every single item in dir(cowsay) and manually excluding the ones you dont want might not be very maintainable in the long term. Is there an easier way to access this information?
implement-cowsay/cow.py
Outdated
| args = parser.parse_args() | ||
|
|
||
| message = " ".join(args.message) | ||
| animal = args.animal or "cow" |
There was a problem hiding this comment.
You are setting a default argument here which is good, but perhaps there may be a neater way of doing this. Think about where the rest of the arguments are handled.
LonMcGregor
added
Reviewed
HassanOHOsman
added
Needs Review
|
Well done, i think this is complete now! |
LonMcGregor
added
Complete
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Self checklist
In this PR I Implemented a command-line tool using the cowsay library with dynamic animal options, message input, validation, and proper --help output.